Now that we have the `MessageQueueNotifierGuard`, we can be sure that we
always dropped locks before notifying. Hence, we can save *a lot* of
error-prone boilerplate that we used to ensure we'd only enqueue if we
dropped locks.

Before e938ed7 and
41f703c, when we had a payment
preimage for a claim which needed to go to a closed channel, we'd
always run the post-`ChannelMonitorUpdate` completion action
immediately as we didn't actually track async
`ChannelMonitorUpdate`s to closed channels. Since those commits we
do, but the comment noting the completion action processing was not
updated.

Thus, here we update the comment, for the easiest close on a major
feature issue ever.

Fixes lightningdevkit#2355.

Client will no longer use a time provider to expire pending requests.
Instead, it will just keep the X most recent requests (coming in next commit).

…eue-notifier

`lightning-liquidity`: Introduce `MessageQueueNotifierGuard` type

per method cooldown enforced correctly, and reset
after peer_connected event

…notification

…lsps5-client-peer-state

[LSPS5 FollowUp] Simplify LSPS5/client peer state

…-notification-cooldown-logic

LSPS5: Correct notification cooldown & reset logic

…comment

Correct post-update action comment on claims from closed chans

In 9cc6e08, we started using the
`RAAMonitorUpdateBlockingAction` logic to block RAAs which may
remove a preimage from one `ChannelMonitor` if it isn't durably
stored in another that is a part of the same MPP claim.

Then, in 254b78f, when we claimed
a payment, if we saw that the HTLC was already claimed in the
channel, we'd simply drop the new RAA blocker. This can happen on
reload when replaying MPP claims.

However, just because an HTLC is no longer present in
`ChannelManager`'s `Channel`, doesn't mean that the
`ChannelMonitorUpdate` which stored the preimage actually made it
durably into the `ChannelMonitor` on disk.

We could begin an MPP payment, have one channel get the preimage
durably into its `ChannelMonitor`, then step forward another update
with the peer. Then, we could reload, causing the MPP claims to be
replayed across all chanels, leading to the RAA blocker(s) being
dropped and all channels being unlocked. Finally, if the first
channel managed to step forward a further update with its peer
before the (now-replayed) `ChannelMonitorUpdate`s for all MPP parts
make it to disk we could theoretically lose the preimage.

This is, of course, a somewhat comically unlikely scenario, but I
had an old note to expand the test and it turned up the issue, so
we might as well fix it.

The ChannelState::NegotiatingFunding assertion check in
ChannelContext::get_initial_commitment_signed will fail when
implementing splicing's channel_reestablish logic. In order to support
it and channel establishment, enter ChannelState::FundingNegotiated
prior to calling the method and update the assertion accordingly.

Also allows writing a channel in the FundedNegotiated state when an
interactive signing session is active. This is necessary as it indicates
a previously funded channel being spliced.

When ChannelContext::get_initial_commitment_signed is called for V2
channel establishment, any errors should result in closing the channel.
However, in the future, when this is used for splicing it should abort
instead of closing the channel. Move the error construction to the call
sites in anticipation of this.

…re-async-tests

If we have pending HTLCs which we intended to forward, but which
were waiting on a `ChannelMonitorUpdate` to be forwarded when we
closed, they will neither be in the `ChannelMonitor` nor in the
`Channel` in a state which indicates they need to be failed (i.e.
in the holding cell). As a result, we previously did not fail such
HTLCs back immediately. Note that we cannot rely on the catch-all
fail-back-before-channel-closure logic either as it is done by the
`ChannelMonitor` that is unaware of these HTLCs.

Here we fix this by detecting the specific case - HTLCs which are
in `LocalSent` (i.e. the counterparty has not provided an RAA yet)
and we have a blocked `ChannelMonitorUpdate` containing a remote
commitment transaction update (which will always contain the HTLC).

In such a case, we can be confident the counterparty does not have
a commitment transaction containing the HTLC, and can fail it back
immediately.

…al-commitment-signed

Support splicing in `ChannelContext::funding_tx_constructed`

Whether it's a splice, or a dual-funded RBF, we need to know which
funding transaction out of all of the negotiated ones is currently
confirmed in case we need to broadcast the holder commitment.

A `FundingScope` can only be promoted once a
`ChannelMonitorUpdateStep::RenegotiatedFundingLocked` is applied, or if
the monitor is no longer accepting updates, once the renegotiated
funding transaction is no longer under reorg risk. Because of this, our
current `FundingScope` may not reflect the latest confirmed state in the
chain. Before making a holder commitment broadcast, we must check which
`FundingScope` is currently confirmed to ensure that it can propogate
throughout the network.

…ing-htlcs-lost-on-mon-delay

Detect and fail-back monitor-blocked un-forwarded HTLCs at close

…commitment-broadcast

Broadcast holder commitment for currently confirmed funding

In 0.1 we started requiring `counterparty_node_id` to be filled in
in various previous-hop datastructures when claiming HTLCs. While
we can't switch `HTLCSource`'s
`HTLCPreviousHopData::counterparty_node_id` to required (as it
might cause us to fail to read old `ChannelMonitor`s which still
hold `HTLCSource`s we no longer need to claim), we can at least
start requiring the field in `PendingAddHTLCInfo` and
`HTLCClaimSource`. This simplifies `claim_mpp_part` marginally.

When we claim a payment, `Event::PaymentClaimed` contains a list of
the HTLCs we claimed from as `ClaimedHTLC` objects. While they
include a `channel_id` the pyment came to us over, in theory
`channel_id`s aren't guaranteed to be unique (though in practice
they are in all opened channels aside from 0conf ones with a
malicious counterparty). Further, our APIs often require passing
both the `counterparty_node_id` and the `channel_id` to do things
to chanels.

Thus, here we add the missing `counterparty_node-id` to
`ClaimedHTLC`.

Historically we indexed channels by
`(counterparty_node_id, funding outpoint)` in several pipelines,
especially the `ChannelMonitorUpdate` pipeline. This ended up
complexifying quite a few things as we always needed to store the
full `(counterparty_node_id, funding outpoint, channel_id)` tuple
to ensure we can always access a channel no matter its state.

Over time we want to move to only the
`(counterparty_node_id, channel_id)` tuple as *the* channel index,
especially as we move towards V2 channels that have a
globally-unique `channel_id` anyway.

Here we take one small step towards this, avoiding using the
channel funding outpoint in the `EventCompletionAction` pipeline.

`ChannelContext::get_pending_htlc_stats` predicts that the set of HTLCs
on the next commitment will be all the HTLCs in
`ChannelContext.pending_inbound_htlcs`, and
`ChannelContext.pending_outbound_htlcs`, as well as all the outbound
HTLC adds in the holding cell.

This is an overestimate:

* Outbound HTLC removals which have been ACK'ed by the counterparty will
  certainly not be present in any *next* commitment, even though they
  remain in `pending_outbound_htlcs` (I refer to states
  `AwaitingRemoteRevokeToRemove` and `AwaitingRemovedRemoteRevoke`).

* Outbound HTLCs in the `RemoteRemoved` state, will not be present in
  the next *local* commitment.

* Inbound HTLCs in the `LocalRemoved` state will not be present in the
  next *remote* commitment.

`ChannelContext::build_commitment_stats(funding, true, true, ..)` makes
these errors when predicting the HTLC count on the remote commitment:

* Inbound HTLCs in the state `RemoteAnnounced` are not included, but
  they will be in the next remote commitment transaction if the local
  ACK's the addition before producing the next remote commitment.

* Inbound HTLCs in the state `AwaitingRemoteRevokeToAnnounce` are not
  included, even though the local has ACK'ed the addition.

* Outbound HTLCs in the state `AwaitingRemoteRevokeToRemove` are
  counted, even though the local party has ACK'ed the removal.

This commit replaces these functions in favor of the newly added
`ChannelContext::get_next_{local, remote}_commitment_stats` methods,
and fixes the issues described above.

We now always calculate dust exposure using a buffer from
`msg.feerate_per_kw`, and not from
`max(feerate_per_kw, self.feerate_per_kw, self.pending_update_fee)`.

Anytime we build a (feerate, nondust-htlc-count, fee) pair, cache it,
and check that the fee matches if the feerate and nondust-htlc-count
match when building a commitment transaction.

The cached fee is never checked in the current test suite.

…mmitment-signed-follow-ups

Follow-ups for lightningdevkit#4014

To align with the "current" and "next" nomenclature used by
HolderCommitmentPoint, update the naming of the counterparty commitment
transaction number field to use "next" instead of "current".

To align with the "current" and "next" nomenclature used by
HolderCommitmentPoint, update the naming of the counterparty commitment
point field to use "next" instead of "current".

To align with the "current" and "next" nomenclature used by
HolderCommitmentPoint, update the naming of the counterparty commitment
point field to use "current" instead of "previous".

The funding inputs used for splicing and v2 channel establishment are
passed as a tuple of txin, prevtx, and witness weight. Add a struct so
that the items included can be better documented.

ChannelManager::splice_channel takes individual parameters to support
splice-in. Change these to an enum such that it can be used for
splice-out as well.

Update SpliceContribution with a variant used to support splice-out
(i.e., removing funds from a channel). The TxOut values must not exceed
the users channel balance after accounting for fees and the reserve
requirement.

When a counterparty sends splice_init with a negative contribution, they
are requesting to remove funds from a channel. Remove conditions
guarding against this and check that they have enough channel balance to
cover the removed funds.

Add splice-out support

When processing a splice_ack, the debug_assert on the range of
our_funding_contribution should account for values that are for both
positive (splice-in) and negative (splice-out).

How fees are paid for in a SpliceContribution depends on whether it is a
SpliceIn or SpliceOut. Include this in its docs for clarification.

[Custom Transactions] Add `TxBuilder::get_next_commitment_stats`

…t-fixups

Follow-ups for lightningdevkit#3979

…ext-counterparty-point

Rename counterparty commitment fields

This simplifies the code and makes it more straightforward to test unblinded
trampoline receives where we need to compute the trampoline session_priv when
manually creating the inner onion. (The trampoline onion needs to be manually
created because LDK does not natively support sending to unblinded trampolines,
just receiving.)

No need to construct unused blinded hop data or hardcode session privs/prng
seeds.

Previously, this test purported to test for a successful and a failing payment
to a single-hop blinded path containing one trampoline node. However, to induce
the failure the test was manually reconstructing the trampoline onion in a
complicated way that encoded the final onion payload as a receive, when for its
purposes it would be simplier for the recipient to just fail the payment
backwards.

In order to not regress in test coverage, the failure method the test was
previously using is re-added in the next commit as a dedicated test.

XXX this new test surfaced a bug that needs to be fixed

This re-adds test coverage for a case that was removed in the previous commit.

This commit adds three new local htlc failure error reasons:
`TemporaryTrampolineFailure`, `TrampolineFeeOrExpiryInsufficient`,
and `UnknownNextTrampoline` for trampoline payment forwarding failures.

We add a `check_trampoline_constraints` similar to
`check_blinded_path_constraints` that compares the Trampoline onion's amount
and CLTV values to the limitations imposed by the outer onion.

Also, we add and modify the following tests:
- Modified the unblinded receive to validate when receiving amount less than
expected.
- Modified test with wrong CLTV parameters that now fails with new enforcement
of CLTV limits.
- Add unblinded and blinded receive tests that forces trampoline onion's CLTV to
be greater than the outer onion packet.

Note that there are some TODOs to be fixed in following commits as we need
the full trampoline forwarding feature to effectively test all cases.

Co-authored-by: Arik Sosman <[email protected]>